Fix slot deploy context value not being used #4617
Conversation
Fixes #4599 - Cannot read properties of undefined (reading 'trim') when backup template version file read returns null/undefined. Added null check before calling toString().trim() on the file content to provide a more actionable error message when backup version files are missing or empty.
- Fixed deploySlot function to properly use expectedContextValue parameter - Added logic to use pickFunctionApp with context filtering when deploying to slots - Ensures Deploy to Slot command only shows slots in picker, not production instances Fixes #4354
There was a problem hiding this comment.
This PR addresses two separate issues:
- Deploy Command Context Value: Fixes slot deployment to properly use the
expectedContextValuefiltering - Template Version Error Handling: Fixes the
Cannot read properties of undefined (reading 'trim')error from Issue #4599
Both fixes are well-implemented and address the root causes correctly. However, there's a missing import for the localize function that needs to be added to prevent compilation errors.
The deploy logic improvement ensures that slot deployments show only deployment slots rather than all function apps, which improves the user experience. The template error handling provides better error messaging and prevents runtime crashes.
Overall good fixes, just needs the missing import resolved.
| const node: SlotTreeItem = await getDeployNode(context, ext.rgApi.tree, arg1, arg2, async () => { | ||
| return await getOrCreateFunctionApp(context) | ||
| }); | ||
| let node: SlotTreeItem; |
There was a problem hiding this comment.
Good: Fixed slot deploy context value usage
The PR correctly addresses the deploy command logic by using expectedContextValue when it's provided and arg1 is undefined. This ensures that slot deploys properly filter to show only deployment slots rather than all function apps.
The addition of pickFunctionApp import and the conditional logic properly implements the intended behavior:
- When deploying to slots (
expectedContextValueis provided), use the filtered picker - Otherwise, use the existing deployment node logic
This fixes the issue where deploySlot command wasn't using its context filtering capability.
Review generated with Copilot
| const versionContent = await AzExtFsExtra.readFile(await this.getBackupVersionPath()); | ||
| if (!versionContent) { | ||
| throw new Error(localize('backupVersionFileEmpty', 'Backup template version file is empty or could not be read')); | ||
| } |
There was a problem hiding this comment.
Good: Fixed template version file error handling
This change properly addresses Issue #4599 where Cannot read properties of undefined (reading 'trim') error occurred when the backup template version file was empty or unreadable.
The fix adds appropriate null/undefined checking before calling .toString().trim(), providing a clear error message when the backup version file is problematic.
The error handling follows good practices:
- Check for falsy values before proceeding
- Throw a localized error message for better user experience
- The error message is actionable (indicates the specific file is empty/unreadable)
Review generated with Copilot
| return (await AzExtFsExtra.readFile(await this.getBackupVersionPath())).toString().trim(); | ||
| const versionContent = await AzExtFsExtra.readFile(await this.getBackupVersionPath()); | ||
| if (!versionContent) { | ||
| throw new Error(localize('backupVersionFileEmpty', 'Backup template version file is empty or could not be read')); |
There was a problem hiding this comment.
Bug: Missing import for localize function
The code uses localize('backupVersionFileEmpty', ...) but there's no import statement for the localize function in this file.
This will cause a compilation error. You need to add:
import { localize } from '../localize';at the top of the file with the other imports.
Review generated with Copilot
Didn't mean to include this.
|
@copilot Fix the linting errors |
Summary
Fixed the
deploySlotcommand to properly use theexpectedContextValueparameter, ensuring that the "Deploy to Slot" command only shows slots in the picker, not production instances.Changes
deployfunction signature to properly useexpectedContextValueinstead of ignoring itpickFunctionAppwith context filtering whenexpectedContextValueis provided and no target node is specifiedpickFunctionApputility functionRoot Cause
The
deploySlotfunction was creating anexpectedContextValueparameter with the correct regex (ResolvedFunctionAppResource.pickSlotContextValue) but thedeployfunction was ignoring this parameter (prefixed with_). This meant that when users ran "Deploy to Slot" from the command palette, they would see all function apps (both production and slots) instead of just slots.Testing
swapSlotFixes #4354